Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changed dir2pi to use original file name in file link. #85

Merged
merged 1 commit into from
Jul 15, 2019

Conversation

tikank
Copy link
Contributor

@tikank tikank commented Dec 4, 2018

As pkg_new_basename was constructed from the normalized package name, it did not always match the original filename. This caused problems with wheels with underscores. Now the original filename (pkg_basename) is used in file link, so the link should match exactly.

I think this change should fix #76 and #84, and is alternative to #67.

I updated also test cases to match this change.

@yjjnls
Copy link

yjjnls commented Dec 5, 2018

pretty good!

@minato7
Copy link

minato7 commented Dec 9, 2018

Yes, this was much needed, any idea in which version this fix will be available?

@RistoVaaraniemi
Copy link

I had to make some ugly local scripting to fix this bug (hopefully temporarily). I hope this project hasn't died.

@longedok
Copy link

longedok commented Mar 5, 2019

I'm facing the same issue, would be grate if those changes could be merged in.

@safiyat
Copy link
Collaborator

safiyat commented Mar 8, 2019

@tikank Sorry for such a late response.

I looked at various PEPs and have concluded that though your code change helps you and others, according to the current standards, all the normalized names contain a hyphen (-) and not an underscore. To quote from the often quoted PEP 503:

This PEP references the concept of a "normalized" project name. As per PEP 426 the only valid characters in a name are the ASCII alphabet, ASCII numbers, ., -, and _. The name should be lowercased with all runs of the characters ., -, or _ replaced with a single - character.

So, the solution you suggested (to use the original filename itself) is not very favorable.

Also, the Python Packaging User Guide suggests that for "Manual" repositories, normalization as defined in PEP 503 should be used.

I am inclined (slightly) to accept this change since it helps so many people, but I will need some good justification for it.

@tikank
Copy link
Contributor Author

tikank commented Mar 8, 2019

The package name is still normalized, and it is used in top level index.html and in directory name. Only file name is used as is. If wheel file name is normalized using the rules described in PEP 503, it is against wheel specification (see PEP 427), and it causes the bugs.
Also, using unchanged file name matches the behavior of pypi.org, for example https://pypi.org/simple/python-dateutil/ .

dtougas pushed a commit to UnivaCorporation/pip2pi that referenced this pull request Mar 25, 2019
dtougas pushed a commit to UnivaCorporation/pip2pi that referenced this pull request Mar 25, 2019
@tikank
Copy link
Contributor Author

tikank commented Mar 30, 2019

Hi there. I am not sure if I can give any more justification for this change. I think that PEP 503 does not say that file names should be normalized:

Below the root URL is another URL for each individual project contained within a repository. The format of this URL is /<project>/ where the <project> is replaced by the normalized name for that project, so a project named "HolyGrail" would have an URL like /holygrail/. This URL must respond with a valid HTML5 page with a single anchor element per file for the project. The text of the anchor tag MUST be the filename of the file and the href attribute MUST be an URL that links to the location of the file for download.

This change does not affect the normalization of the /<project>/ part.

Looking the example from Python Packaging User Guide, the Foo-1.0.tar.gz file name is not either normalized, only the directory name is.

Also as the main use case for the pip2pi is to download packages and form package repository, I think it is safe to assume the file names are already in correct format, and pip2pi should use them without change.

I hope this helps to decide whether the pull request should be accepted or rejected.

@terrdavis
Copy link

Thanks @tikank. I'll be using your fork until this pull request is accepted.

@kirkfox
Copy link

kirkfox commented Sep 10, 2019

Is this going to get published to pypi or are we required to clone the repo?

@terrdavis
Copy link

I've added the commit archive url: https://github.com/wolever/pip2pi/archive/387105139257cff8a407eaf873c7ee778b4e26ea.zip
to my requirements file.
It's pinned to the merge-commit, so I won't see any further changes.
It would be much appreciated if they published this to pypi though!

@safiyat
Copy link
Collaborator

safiyat commented Sep 28, 2019

pip2pi 0.8.1 released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dir2pi normalization broken for some wheel packages
8 participants